feat(ui): add StreamContextMenu and StreamContextMenuItem#53
feat(ui): add StreamContextMenu and StreamContextMenuItem#53xsahil03x merged 5 commits intomain-design-systemfrom
Conversation
- Add `PlatformSelector` component to switch between target platforms (Android, iOS, etc.). - Update `PreviewConfiguration` to store and notify platform changes. - Apply platform override in `PreviewWrapper` to ensure `StreamTheme` and `Theme` reflect the selected platform. - Add the selector to the main `Toolbar`.
… playground examples
📝 WalkthroughWalkthroughAdds a new StreamContextMenu component and StreamContextMenuItem with full theming, integrates them into StreamTheme and component factory, exposes components in public exports, updates the design gallery with playground/showcase demos, and adds a platform override UI and plumbing in the gallery preview. Also refactors button theming to support icon sizing. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Toolbar
participant PreviewConfig
participant PreviewWrapper
participant StreamTheme
User->>Toolbar: selects platform in PlatformSelector
Toolbar->>PreviewConfig: setTargetPlatform(platform)
PreviewConfig-->>Toolbar: notify listeners
PreviewWrapper->>PreviewConfig: reads targetPlatform
PreviewWrapper->>PreviewWrapper: wrap content with _PlatformOverride
_PlatformOverride->>StreamTheme: applyPlatform(platform)
StreamTheme-->>PreviewWrapper: themed content
PreviewWrapper->>User: renders preview for selected platform
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (30.18%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main-design-system #53 +/- ##
=====================================================
Coverage ? 35.22%
=====================================================
Files ? 95
Lines ? 2836
Branches ? 0
=====================================================
Hits ? 999
Misses ? 1837
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
packages/stream_core_flutter/lib/src/components/buttons/stream_button.dart (1)
376-647: Consider reducing boilerplate across the 9 default classes.All 9 classes share identical structure: constructor capturing
context,colorScheme,isFloating, with the mixin providing shared defaults. The only variance is inbackgroundColor,foregroundColor, andborderColor. You could extract a common abstract base that handles the constructor boilerplate, with subclasses only overriding the color properties.That said, the current approach is explicit and easy to follow — this is a low-priority suggestion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stream_core_flutter/lib/src/components/buttons/stream_button.dart` around lines 376 - 647, Multiple nearly-identical classes (_PrimarySolidDefaults, _PrimaryOutlineDefaults, _PrimaryGhostDefaults, _SecondarySolidDefaults, _SecondaryOutlineDefaults, _SecondaryGhostDefaults, _DestructiveSolidDefaults, _DestructiveOutlineDefaults, _DestructiveGhostDefaults) repeat constructor and fields; extract a single abstract base (e.g., _BaseButtonDefaults extends StreamButtonThemeStyle with _SharedButtonDefaults) that takes BuildContext and isFloating, assigns colorScheme = context.streamColorScheme, and provides helper protected methods for the common WidgetStateProperty.resolveWith patterns (e.g., resolveBackground(base), resolveForeground(defaultColor), resolveBorder(defaultColor)); then make each specific class minimal by extending _BaseButtonDefaults and only overriding backgroundColor/foregroundColor/borderColor to call those helpers or supply their differing base colors (e.g., colorScheme.accentPrimary, colorScheme.backgroundSurface, colorScheme.accentError, isFloating logic, and StreamColors.transparent) so the constructor/field boilerplate is removed from each subclass.packages/stream_core_flutter/lib/src/components/context_menu/stream_context_menu.dart (1)
192-215: Consider avoiding storingBuildContextin a field.Storing
BuildContextin_ContextMenuStyleDefaultsworks because it's created and consumed within a singlebuild()call, but it's a pattern that Flutter lint rules sometimes flag. An alternative is to pass the resolved theme primitives directly. That said, this is a private class with a tightly scoped lifecycle, so the current approach is acceptable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stream_core_flutter/lib/src/components/context_menu/stream_context_menu.dart` around lines 192 - 215, Avoid storing BuildContext in _ContextMenuStyleDefaults: change its constructor to accept the resolved theme primitives (StreamRadius, StreamSpacing, StreamBoxShadow, StreamColorScheme) instead of BuildContext and remove the context field; replace the late final fields (_radius, _spacing, _boxShadow, _colorScheme) with constructor-assigned fields and update any callers (where _ContextMenuStyleDefaults(...) is created inside build()) to compute context.streamRadius, context.streamSpacing, context.streamBoxShadow and context.streamColorScheme and pass them in.packages/stream_core_flutter/lib/src/components/context_menu/stream_context_menu_item.dart (1)
219-227:MainAxisSize.minis a no-op whenExpandedis present.
Expandedforces theRowto consume all available main-axis space, soMainAxisSize.minon Line 221 has no practical effect. This is functionally harmless (the parentTextButtonprovides bounded constraints), but it could mislead future readers into thinking the row shrinks to its content.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stream_core_flutter/lib/src/components/context_menu/stream_context_menu_item.dart` around lines 219 - 227, The Row sets mainAxisSize: MainAxisSize.min while also wrapping props.label in Expanded, which makes mainAxisSize a no-op; remove the misleading property (mainAxisSize: MainAxisSize.min) from the Row in StreamContextMenuItem (the widget that builds the Row with ?props.leading, Expanded(child: props.label), ?props.trailing) so the layout is clear, or alternatively replace Expanded with Flexible if the intent is to allow the Row to size to its content.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/design_system_gallery/lib/core/preview_wrapper.dart`:
- Around line 120-124: The overriddenStreamTheme construction discards
component-level customizations by only passing brightness, platform, and
colorScheme into the StreamTheme constructor; update the overridenStreamTheme
creation to forward all component theme properties from currentStreamTheme
(e.g., avatarTheme, badgeCountTheme, buttonTheme, contextMenuTheme,
contextMenuItemTheme, emojiButtonTheme, messageTheme, inputTheme,
onlineIndicatorTheme, onlineIndicatorTheme, etc.) while keeping brightness,
platform and colorScheme recalculated so existing per-component customizations
are preserved when platform-aware primitives (radius/typography) are recomputed.
In
`@packages/stream_core_flutter/lib/src/components/context_menu/stream_context_menu_item.dart`:
- Around line 279-287: The padding and shape getters use dot-shorthand on
abstract target types which resolves to nonexistent static members; in the
padding getter (WidgetStateProperty<EdgeInsetsGeometry> get padding) replace
`.symmetric(...)` with the concrete constructor EdgeInsets.symmetric(...), and
in the shape getter (WidgetStateProperty<OutlinedBorder> get shape) replace
`.all(...)` with the concrete BorderRadius.all(...), keeping the surrounding
WidgetStatePropertyAll and RoundedRectangleBorder usage intact so the types
resolve correctly.
In
`@packages/stream_core_flutter/lib/src/theme/components/stream_context_menu_theme.g.theme.dart`:
- Around line 154-186: The equality and hashCode for StreamContextMenuStyle use
list identity for boxShadow (in operator== and hashCode), causing incorrect
comparisons; update the generated code in ThemeGenGenerator so operator==
compares lists with listEquals(_other.boxShadow, _this.boxShadow) (importing
package:flutter/foundation.dart) and change the hashCode to include a deep hash
for the list (e.g., Object.hashAll(_this.boxShadow) or similar) instead of
hashing the list object itself; ensure references to backgroundColor, shape,
side, boxShadow, padding remain and only the boxShadow comparison/hash logic is
adjusted.
---
Nitpick comments:
In `@packages/stream_core_flutter/lib/src/components/buttons/stream_button.dart`:
- Around line 376-647: Multiple nearly-identical classes (_PrimarySolidDefaults,
_PrimaryOutlineDefaults, _PrimaryGhostDefaults, _SecondarySolidDefaults,
_SecondaryOutlineDefaults, _SecondaryGhostDefaults, _DestructiveSolidDefaults,
_DestructiveOutlineDefaults, _DestructiveGhostDefaults) repeat constructor and
fields; extract a single abstract base (e.g., _BaseButtonDefaults extends
StreamButtonThemeStyle with _SharedButtonDefaults) that takes BuildContext and
isFloating, assigns colorScheme = context.streamColorScheme, and provides helper
protected methods for the common WidgetStateProperty.resolveWith patterns (e.g.,
resolveBackground(base), resolveForeground(defaultColor),
resolveBorder(defaultColor)); then make each specific class minimal by extending
_BaseButtonDefaults and only overriding
backgroundColor/foregroundColor/borderColor to call those helpers or supply
their differing base colors (e.g., colorScheme.accentPrimary,
colorScheme.backgroundSurface, colorScheme.accentError, isFloating logic, and
StreamColors.transparent) so the constructor/field boilerplate is removed from
each subclass.
In
`@packages/stream_core_flutter/lib/src/components/context_menu/stream_context_menu_item.dart`:
- Around line 219-227: The Row sets mainAxisSize: MainAxisSize.min while also
wrapping props.label in Expanded, which makes mainAxisSize a no-op; remove the
misleading property (mainAxisSize: MainAxisSize.min) from the Row in
StreamContextMenuItem (the widget that builds the Row with ?props.leading,
Expanded(child: props.label), ?props.trailing) so the layout is clear, or
alternatively replace Expanded with Flexible if the intent is to allow the Row
to size to its content.
In
`@packages/stream_core_flutter/lib/src/components/context_menu/stream_context_menu.dart`:
- Around line 192-215: Avoid storing BuildContext in _ContextMenuStyleDefaults:
change its constructor to accept the resolved theme primitives (StreamRadius,
StreamSpacing, StreamBoxShadow, StreamColorScheme) instead of BuildContext and
remove the context field; replace the late final fields (_radius, _spacing,
_boxShadow, _colorScheme) with constructor-assigned fields and update any
callers (where _ContextMenuStyleDefaults(...) is created inside build()) to
compute context.streamRadius, context.streamSpacing, context.streamBoxShadow and
context.streamColorScheme and pass them in.
packages/stream_core_flutter/lib/src/components/context_menu/stream_context_menu_item.dart
Show resolved
Hide resolved
packages/stream_core_flutter/lib/src/theme/components/stream_context_menu_theme.g.theme.dart
Show resolved
Hide resolved
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/design_system_gallery/lib/core/preview_wrapper.dart`:
- Around line 98-122: The _PlatformOverride build currently replaces the prior
broken logic by using streamTheme.applyPlatform(platform); ensure this stays and
modify the extensions argument to pass a List combining existing extensions with
the platform-applied StreamTheme (e.g., use [...theme.extensions.values,
streamTheme.applyPlatform(platform)]) so ThemeData.copyWith(extensions: ...)
receives the correct ordered collection and the StreamTheme from
streamTheme.applyPlatform(platform) overrides the existing StreamTheme; key
symbols: _PlatformOverride, streamTheme, applyPlatform,
ThemeData.copyWith(extensions:).
Summary by CodeRabbit
New Features
Documentation